Skip to content

chore: metric-view runtime#474

Draft
atilafassina wants to merge 9 commits into
mainfrom
mv-runtime
Draft

chore: metric-view runtime#474
atilafassina wants to merge 9 commits into
mainfrom
mv-runtime

Conversation

@atilafassina

@atilafassina atilafassina commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Adds the runtime half of the Metric Views feature: a POST /api/analytics/metric/:key route that measures a registered UC Metric View, mirroring the existing /query SSE envelope. Dormant until an app adds config/queries/metric-views.json.

Builds on the landed typegen/config PRs (PR0/1/3). This branch is the full PR2 stack (phases 1–3) plus a round of adversarial-review hardening.

What's here

Feature (phases 1–3)

  • Metric-view route skeleton + measures-only SQL (MEASURE(m) AS m)
  • Dimensions + GROUP BY ALL; structured recursive filter engine (12 operators, parameterized :f_<idx> binds, AND/OR groups with depth + cardinality caps)
  • timeGrain bucketing via an explicit timeDimension (date_trunc)
  • Lane dispatch from the entry's executor (SP shared-cache vs OBO per-user cache); cache-key composition + per-user identity hashing

Adversarial-review fixes

  • Registry lookup hardening — null-prototype registry + Object.hasOwn read, so a key colliding with an inherited Object.prototype member can't bypass the 404
  • Measure/dimension uniqueness — reject duplicates and measure∩dimension collisions that would silently clobber row-object keys during materialization
  • Filter sort-key delimiter/-delimited (member, operator) sort key so distinct pairs can't collide and fork the cache on equivalent filters
  • Format guard — reject non-JSON_ARRAY formats (Arrow returned JSON silently before)
  • FQN grammar unification — one grammar + one escaper across schema, typegen, and runtime: isValidFqn + quoteFqnForSql moved into the shared zod-free leaf; the runtime now validates and backtick-quotes the FQN instead of applying a narrow ASCII allowlist and interpolating unquoted. Fixes a latent break where a documented-legal UC name (e.g. prod-data.analytics.revenue) passed config + typegen but failed at runtime.
  • Registry loading — lazy + async with an mtime-validated module cache (keyed by dir), replacing the permanent success/failure memo. Matches the sibling .sql path's re-read/self-heal behavior at a lower steady-state cost (one stat, no re-parse when unchanged): a fixed config self-heals, an edit hot-reloads, and the blocking readFileSync is gone.
  • Cache-key hardening — salt on source (repoint isn't stale-served); only salt timeDimension when timeGrain is set

Testing

vitest — metric (128) + mv-registry + shared + analytics suites green; tsc --noEmit clean; Biome clean. Registry loading is exercised through real temp-dir config files (no private-state poking).


This pull request and its description were written by Isaac.

…se 1)

xavier loop: iteration 1 — Phase 1 walking skeleton.

POST /api/analytics/metric/:key over the standard SSE envelope, SP lane only.
Synchronous config-parse registration from config/queries/metric-views.json
against the landed metricSourceSchema (single metricViews map; lane derived
from executor). Measures-only buildMetricSql (SELECT MEASURE(m) AS m FROM
<fqn> [LIMIT n]) gated by MEASURE_NAME_PATTERN + assertSafeFqn — grammar gate
only, no name allowlist. 503 METRIC_REGISTRY_LOAD_FAILED on malformed config,
404 on unknown key (generic public bodies; detail to telemetry).

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
… 2a)

xavier loop: iteration 2 — Phase 2 dimensions + filter engine.

GROUP BY ALL for dimensions; recursive 12-operator filter engine
(equals/notEquals/in/notIn/gt/gte/lt/lte/contains/notContains/set/notSet)
with every value bound as a :f_<idx> named parameter — never interpolated.
member/dimension gated by DIMENSION_NAME_PATTERN; AND/OR composition; depth
capped at 8, enforced twice (iterative pre-Zod preCheckFilterDepth + renderer
re-check). Static (registration-free) request schema: operator enum,
per-operator value cardinality, group/values/limit caps. No name allowlist.

timeGrain application deliberately held (parsed + grammar-gated, not yet
applied) pending the grain-target decision — lands in phase 2b.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
xavier loop: iteration 2 — Phase 2 grain slice.

Resolves the grain-target gap: PR2 drops the registry that #341 used to infer
which dimension was temporal, so the grain's target is named explicitly. New
optional `timeDimension` request field; when `timeGrain` is set, that column
renders as date_trunc('<grain>', <col>) AS <col> (grain single-quoted literal
gated by TIME_GRAIN_PATTERN; column gated by DIMENSION_NAME_PATTERN — neither
can be a bind param). Other dimensions render bare. Two 400 rules in the
request schema superRefine: timeGrain without timeDimension; timeDimension not
in dimensions. Warehouse remains the authority on column temporality.

Deviates from the PRD's "date_trunc on time-typed dimensions" wording (which
presupposed the deleted registry) — the explicit-field shape is the runtime
query shape PR2 settles; PR5's hook contract inherits the timeDimension arg.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
xavier loop: iteration 3 — Phase 3, completes PR2.

Lane dispatch from the registration (metric-views.json executor), not a URL
segment: OBO-lane metrics run on-behalf-of the requesting user via asUser(req)
with a per-user cache scope; SP-lane metrics run as the app service principal
with a shared scope. Executor + key computed inside a try so a missing/
whitespace OBO identity (from asUser/resolveUserId/deriveMetricExecutorKey)
lands on the canonical 401 envelope, not an uncaught 500.

composeMetricCacheKey builds metric:{key}:{argsHash}:{executorKey} over
canonicalized args (sorted measures/dimensions, order-independent filter
fingerprint via canonicalizeFilter, grain, timeDimension, limit).
deriveMetricExecutorKey returns "sp" for SP and a sha256 of the trimmed user
identity for OBO — the raw email/principal never enters the cache layer.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
…rmat

Four defensive fixes to the PR2 metric-view runtime surfaced by adversarial
review:

- Registry lookup (B): build the registry with a null prototype and gate the
  route read with Object.hasOwn, so a metric key colliding with an inherited
  Object.prototype member (__proto__, constructor, toString, …) can no longer
  resolve to a truthy non-registration and bypass the unknown-key 404.

- Measure/dimension uniqueness (C): reject a name that repeats within measures,
  within dimensions, or across both. Measures and dimensions alias to their own
  name in the SELECT list (MEASURE(x) AS x, x), so a duplicate collapses to one
  row-object key and silently drops a value during row materialization.

- Filter sort key (D): delimit the (member, operator) sort key with "/" instead
  of a bare concatenation, so two distinct pairs cannot map to the same key and
  fork the cache on semantically equal filters. Matches the delimiter
  canonicalizeFilter already uses for its leaf fingerprint.

- Format (F): reject any format other than JSON_ARRAY (legacy aliases
  normalized first). The metric route delivers JSON rows only at v1; an Arrow
  request previously returned JSON silently instead of failing loud.

Adds 10 tests covering all four.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
The runtime, the shared schema, and the type-generator disagreed on what a
metric-view `source` FQN may contain. The shared schema and typegen accept the
full UC quoted-identifier grammar (hyphens, non-ASCII) and typegen backtick-
quotes before interpolation; the runtime used a narrower ASCII allowlist
(assertSafeFqn) AND interpolated the FQN UNQUOTED. So a documented-legal UC
name like `prod-data.analytics.revenue` passed config + type generation but
threw (or, if the pattern were widened, emitted invalid SQL) at request time.

Converge on one grammar + one escaper:

- Move `isValidFqn` (three-part UC predicate) and `quoteFqnForSql` (backtick
  escaper) into the shared zod-free leaf `metric-fqn.ts`, beside
  `UC_FQN_PATTERN`. Grammar and quoting now have a single home both the
  type-generator and the analytics runtime import. `describe.ts` and
  `config.ts` import them back; `mv-registry.test.ts` imports the escaper from
  the leaf.
- Runtime `buildMetricSql` replaces the narrow `assertSafeFqn` regex with
  `quoteSafeFqn`: validate via `isValidFqn`, then interpolate the
  `quoteFqnForSql`-escaped FQN. Quoting is the injection boundary, so the
  runtime accepts exactly what UC (and the schema, and typegen) accept.
- `UC_THREE_PART_FQN_PATTERN` stays in `metric-source.ts` so zod still emits a
  JSON-schema `pattern`; it derives from the same per-segment charset as
  `isValidFqn`, so the two shapes cannot diverge.

Also folds in cache-key hardening (in-flight): salt the metric cache key with
`source` so repointing a metric key to a different FQN cannot stale-serve, and
only salt `timeDimension` when `timeGrain` is set (it has no SQL effect
otherwise). `renderDimensionClause` re-gates the grain against
TIME_GRAIN_PATTERN at its interpolation point.

Tests: requote the ~13 emitted-FROM assertions; add regression tests that a
hyphenated FQN is accepted+quoted and that a backtick-bearing segment is
neutralized by doubling.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
@github-actions

Copy link
Copy Markdown
Contributor

🔬  Run evals on this PR  ·  Go to Evals Monitor →

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 737 KB (+35 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 770 KB (+41 KB) 269 KB (+14 KB)
Type declarations 276 KB (+1.8 KB) 94 KB (+756 B)
Source maps 1.5 MB (+79 KB) 499 KB (+25 KB)
Other 11 KB 3.7 KB
Total 2.5 MB (+122 KB) 866 KB (+40 KB)
Per-entry composition (own code — deps external (as shipped))
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
. 84 KB (+5.2 KB) 2.5 KB 87 KB (+5.2 KB) external 275 KB (+17 KB)
./beta 39 KB 230 B 39 KB external 117 KB
./type-generator 19 KB (+18 B) 0 B 19 KB (+18 B) external 54 KB (+13 B)

Chunks:

Entry Chunk Load Size (gz)
. index.js initial 80 KB
. utils.js initial 4.0 KB
. remote-tunnel-manager.js lazy 2.5 KB
./beta beta.js initial 30 KB
./beta databricks.js initial 5.7 KB
./beta service-context.js initial 3.2 KB
./beta client-options.js initial 220 B
./beta databricks.js lazy 127 B
./beta index.js lazy 103 B
./type-generator index.js initial 19 KB

@databricks/appkit-ui

npm tarball (packed): 295 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 350 KB 116 KB
Type declarations 201 KB 72 KB
Source maps 669 KB 218 KB
CSS 16 KB 3.3 KB
Total 1.2 MB 410 KB
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
./js 4.3 KB 49 KB 54 KB 208 KB 12 KB
./js/beta 20 B 0 B 20 B 0 B 0 B
./react 428 KB 49 KB 476 KB 1.3 MB 163 KB
./react/beta 20 B 0 B 20 B 0 B 0 B

Chunks:

Entry Chunk Load Size (gz)
./js index.js initial 4.2 KB
./js chunk initial 120 B
./js apache-arrow lazy 49 KB
./js/beta beta.js initial 20 B
./react index.js initial 426 KB
./react tslib initial 2.1 KB
./react apache-arrow lazy 49 KB
./react/beta beta.js initial 20 B

The metric registry memoized both success AND failure on the first
`/metric/:key` request and never re-read: editing `metric-views.json` needed a
server restart, and a transient first-request error latched a 503 forever —
unlike the sibling `.sql` query path, which re-reads `config/queries/` per
request. It was also a synchronous `readFileSync`, blocking the event loop for
every request (metric or not) under load.

Match the `.sql` path's behavior, then beat its per-request cost:

- `loadMetricRegistry` is now async (`fs.promises`) and stays a pure, stateless
  parse. Metric views are already heavier than a plain query on the warehouse
  side, so the SDK layer must not add a blocking read.
- New `getMetricRegistry(dir)` wraps it with a module-level cache keyed by the
  queries DIR (not the plugin instance): the registry is a pure function of the
  config file, warehouse-independent, so two plugins at one dir share one parse.
  Each request does a single async `stat`; the read + JSON.parse + zod
  validation are skipped when the file's (mtimeMs, size) signature is unchanged
  — steady-state cost is below the `.sql` path (a stat, not a full read).
- Failures are NOT cached (cache populated only on a successful parse), so a
  fixed config self-heals on the next request; an edit bumps mtime so a working
  config hot-reloads; an absent file stays dormant (ENOENT → empty registry).
- Deletes the `metricRegistry` / `metricRegistryLoadError` fields and the
  `_getMetricRegistry` memo; the route calls `getMetricRegistry` in a try/catch
  → 503 on throw.

Registry loading is now exercised through real files: `AnalyticsPlugin` takes a
test-only `queriesDir` config (documented `@internal`), so tests point the
loader at a temp dir and drive the real stat→read→cache path instead of poking
private state. Removes the `setRegistry` backdoor. Adds hot-reload, self-heal,
and mtime-cache tests.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
@atilafassina atilafassina changed the title PR2: metric-view runtime (route + SQL builder + filter engine + review fixes) chore: metric-view runtime Jul 10, 2026
…-4 gaps

Third adversarial review round (two reviewers, deduped). Highest-priority
finding: the FQN grammar drift fixed earlier was still present for
measures/dimensions/filter-members — the type-generator emits DESCRIBE column
names verbatim into the generated measureKeys/dimensionKeys unions, but the
runtime gated them on the narrow /^[a-zA-Z_][a-zA-Z0-9_]*$/ and interpolated
bare, so a UC column like `net-revenue` / `café_sales` typechecked yet 500'd at
runtime (generate-but-500, same class as the FQN bug).

A+C — quote column identifiers + validate early:
- Add `isValidColumnName` + `quoteIdentifier` to the shared zod-free leaf.
  `quoteIdentifier` is the single-identifier escaper (does NOT split on `.`, so
  a column literally named `net.revenue` becomes one delimited identifier);
  `quoteFqnForSql` now maps it over dot-split segments. The column grammar is
  the full delimited-identifier set — reject only control/newline (what cannot
  be safely quoted), matching exactly what typegen can emit.
- Runtime backtick-quotes measures (MEASURE(`x`) AS `x`), dimensions, the
  date_trunc column, and filter members. Quoting — not a narrow allowlist — is
  the injection boundary, so an injection-shaped name is neutralized (inert
  quoted column), not rejected. Row-key preservation holds: the warehouse
  reports the aliased column under the unquoted name, so `{ "net-revenue": … }`.
- Move identifier validation into `validateMetricRequest` (via a refine on
  measures/dimensions/timeDimension/filter.member) so a malformed identifier
  returns the canonical 400 instead of failing inside the SSE execute path as a
  retried 500 (finding C). The builder keeps its checks as defense-in-depth.
- Delete the now-unused MEASURE_NAME_PATTERN / DIMENSION_NAME_PATTERN.
- Fixed a regression this introduces: the filter sort key and canonicalize
  fingerprint used a "/" delimiter that was safe only while members couldn't
  contain "/"; now that members accept the full grammar, both JSON-encode the
  tuple so distinct (member, operator[, values]) pairs can't collide and
  fork/merge cache entries.

B — registry cache signature adds ctimeMs (from the same stat): a same-size,
same-mtime edit (equal-length source/executor swap on a coarse-mtime FS) now
invalidates, closing a stale-source/stale-lane serve. + same-size regression
test.

D — runtime config caps now match the type-generator: MAX_METRIC_VIEWS (200)
and per-segment length (255) enforced via the shared schema's superRefine, plus
a declarative maxLength (767) on `source` (the only cap `z.toJSONSchema` can
serialize — regenerated metric-source.schema.json). Refinements are invisible
to JSON-schema generation, so runtime + typegen stay the authoritative gates.

E — export test-only `__resetMetricRegistryCache` so cache isolation between
tests is intentional, not an accident of unique temp dirs. F — comment that a
non-ENOENT stat error is deliberately fatal-per-request (self-heals next call).

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>
Signed-off-by: Atila Fassina <atila@fassina.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant